Skip to content

feat(middleware): optional RateLimiterStoreContext for response headers (#2961)#3007

Open
vishr wants to merge 2 commits into
masterfrom
fix-2961-ratelimit-context
Open

feat(middleware): optional RateLimiterStoreContext for response headers (#2961)#3007
vishr wants to merge 2 commits into
masterfrom
fix-2961-ratelimit-context

Conversation

@vishr

@vishr vishr commented Jun 13, 2026

Copy link
Copy Markdown
Member

Problem (#2961)

The rate limiter sets no Retry-After / X-RateLimit-* headers, and the RateLimiterStore interface (Allow(identifier) (bool, error)) gives a store no way to set them either.

Fix

Add an optional interface:

type RateLimiterStoreContext interface {
	AllowContext(c *echo.Context, identifier string) (bool, error)
}

When the configured store implements it, the middleware calls AllowContext (with the request context) instead of Allow, so the store can set response headers on the allow/deny decision.

Fully backward compatible — stores implementing only Allow are unaffected; the existing interface and the built-in memory store are unchanged.

This is the optional-interface approach @aldas proposed in the issue thread (mirroring the pattern used in the v4 proxy middleware). It intentionally does not retrofit the built-in store with full metadata plumbing (the part flagged as a larger rewrite in the thread) — it just provides the hook so stores can set headers.

Test

TestRateLimiter_storeAllowContextIsPreferred (written first; fails before the change): a store implementing AllowContext is preferred over Allow and can set a Retry-After header. gofmt/vet clean; full rate-limiter suite passes (backward compat confirmed).

Addresses #2961.

🤖 Generated with Claude Code

…rs (#2961)

Adds an optional RateLimiterStoreContext interface. When the configured store
implements AllowContext(c, identifier), the rate limiter calls it instead of
Allow, giving the store access to the request context so it can set response
headers such as Retry-After / X-RateLimit-*. Fully backward compatible: stores
implementing only Allow are unchanged.

This is the optional-interface approach proposed by the maintainer in the issue
thread; it does not alter the existing Allow interface or the built-in store.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aldas

aldas commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Note: this is same (subset) change as #2985 does but @leno23 created that PR against v4.

…#2961)

Implements AllowContext on RateLimiterMemoryStore so the default store sets
X-RateLimit-Limit, X-RateLimit-Remaining, and (on deny) Retry-After headers
out of the box — mirroring the v4 PR #2985 by @leno23 on the v5 line.

Allow() is refactored to share an internal allow() with AllowContext; the
optional RateLimiterStoreContext interface (added earlier in this PR) routes
the middleware to AllowContext when the store implements it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vishr

vishr commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

Thanks @aldas — you're right, this is the v5 counterpart of #2985. I've expanded it to mirror that PR: RateLimiterMemoryStore now implements AllowContext and sets X-RateLimit-Limit / X-RateLimit-Remaining (and Retry-After on deny) out of the box, so the default store gets headers without a custom store — same behavior as #2985, ported to the v5 *echo.Context API.

One small divergence: the optional interface is exported here (RateLimiterStoreContext) rather than unexported as in #2985, so custom-store authors on v5 can reference the type directly — happy to make it unexported to match if you'd prefer them identical. If #2985's design shifts during review, I'll keep this one aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants